-
Notifications
You must be signed in to change notification settings - Fork 364
Add linspace_exclusive function.
#1580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Add a `linspace_exclusive` function which creates a `Linspace` iterator which is exclusive at its upper bound. This is essentially the same as `linspace(endpoint=False)` in numpy.
|
If this is a welcome addition I'll add some tests and an equivalent for I thought about using I think this is nicer and more idiomatic, but I don't see a good way of adding it that doesn't also break backwards compatibility. |
Interesting. I do find it nicer, but this is only my opinion. What's cool is that the same name could be reused instead of creating new names like |
|
Strongly agree, I love this approach, so my question is how we can make a smooth transition (or we'll just have to make a warning somehow?) |
|
Since Rust doesn't do function overloading there really are only two options:
I don't know how opposed you are to these kinds of API changes, I'll leave it to you to decide. |
|
I'm in favor of just doing a breaking change - I generally feel that we should take advantage of our 0.* status and move quickly towards 1.0, which is my real North Star for the library. I might look into some sort of upgrade script, since this is a pretty simple fix. But it's probably not worth the time. For now, can you please alter this PR to change |
akern40
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work here! One quick note, and then a bigger idea / comment. Our formatting rules require running your formatter on +nightly; I'm guessing that's why many of the files have formatting changes.
Looking over the linspace and logspace implementations, it didn't occur to me that we'd have to handle ranges like .., ..b, and a... Panicking is certainly one way out of that, but I'd prefer not to add panic conditions to the library. That leaves us with two three choices, that I see:
- Go back to the
linspaceandlinspace_inclusivedesigns - Use a custom trait,
FiniteRange<F>: RangeBounds<F>, implemented only forRangeandRangeInclusive - Switch to
try_linspaceinstead oflinspace, and returnNoneif the range isn't finite.
Pro of option 2 is that the invariant is checked at compile time; con is that it will require users to import the trait. Pro of option 3 is that users don't have to import the trait, but con is that they need to check the values at runtime.
Personally I'd vote for Option 2, but I'm curious to hear if people feel differently.
Add a
linspace_exclusivefunction which creates aLinspaceiterator which is exclusive at its upper bound. This is essentially the same aslinspace(endpoint=False)in numpy.Addresses #1169